Repository navigation
Conversation
Buffer had no own `of`, so `Buffer.of` was the inherited %TypedArray%.of. That function constructs through `this`. A detached call threw "TypedArray.of requires |this| to be a constructor", `Buffer.of.call(Uint8Array, ...)` returned a Uint8Array, and a subclass ran its constructor. Node defines its own `of` as an arrow function (lib/buffer.js). Add one row to jsBufferConstructorTable and a host function that allocates a Buffer of argumentCount bytes and stores each argument in order.
|
No actionable comments were generated in the recent review. WalkthroughThe Buffer constructor now exposes ChangesBuffer.of
Suggested reviewers: Merge Risk:
|
|
Status Reproduced on main (017b243) and on Bun 1.4.3 with: const { of } = Buffer;
console.log(of(1, 2));
// main: TypeError: TypedArray.of requires |this| to be a constructor
// this PR: <Buffer 01 02>
// Node v26.3.0: <Buffer 01 02>
The branch contains main 2596ac5 (head 367f42f) and has no conflict with it. CI on 367f42f (build 138891): 183 of the 184 jobs that ran pass. Every build job, The one failed job is a test shard of The diff is ready for review. For a green status, retry the failed shard and the 4 expired jobs. Pull request: #45865 |
There was a problem hiding this comment.
LGTM — a small, self-contained addition that matches Node's Buffer.of and follows the existing static-table pattern in this file.
What was reviewed:
- The new host function in src/jsc/bindings/JSBuffer.cpp: throw scope is declared up front,
RETURN_IF_EXCEPTIONfollows both the allocation and eachsetIndex, anduncheckedArgument(i)is bounded byargumentCount(). - Reentrant
valueOf/toStringduringsetIndex: the buffer is unreachable from JS until return, and JSC'ssetIndexcoerces first and re-checks detached/bounds afterwards, so user code cannot corrupt the write. - Realm handling:
createUninitializedBufferresolves the structure throughdefaultGlobalObject, the same as every sibling static (alloc,concat, ...), so this does not introduce a new inconsistency. - The test block imports
vmfrom the existing file header, covers receiver variants, coercion order, throw propagation, the 1000/1001 inline-storage boundary, and GC insidevalueOf.
Extended reasoning...
The diff adds one 15-line host function and one lookup-table row to src/jsc/bindings/JSBuffer.cpp, plus a new describe block in test/js/node/buffer.test.js; nothing else changes. It touches no security-sensitive surface (no parsing of untrusted sizes beyond the argument count, no auth or crypto). The function mirrors the structure of the neighbouring statics, has exception checks at every point that can enter user JS, and the behaviour change is deliberately aligned with Node and documented in the PR. No CODEOWNERS entry covers the changed files, the hunt ran dry with no findings, and there are no third-party objections in the timeline.
|
Updated 5:05 PM PT - Nov 8th, 2026
❌ @robobun, your commit 367f42f has 1 failures in
🧪 To try this PR locally: bunx bun-pr 45865That installs a local version of the PR into your bun-45865 --bun |
The table of built-ins holds each row of a native property table against src/codegen/builtins/data/own-properties.txt. This branch adds the row `of` to the Buffer constructor, so the list gets `of/wc` (writable and configurable), like the other functions of that table.
|
Merged main (11c3671) into this branch. The merge has no conflict, but the merged tree did not build. The step that generates the table of built-ins stopped:
|
…d classes test/aot/programs/language/typed-array-and-buffer-classes-keep-their-members listed `of` among the keys that Buffer inherits and is closed for. This branch makes it an own function of Buffer, so a compiled program locks it like `from`: a `delete` throws where it returned true before. The program lists `of` as a member, and the stored output follows.
|
One more edit after the merge of main: a program of
|
The row `of` of jsBufferConstructorTable had no declaration in buffer.builtins.ts. The table of built-ins gave it the overload of an undeclared row: any result, and any use of its operands. Declare what the host function does. It takes any number of operands, runs only their ToPrimitive, and answers a fresh Buffer. The other ten statics of that table already have a declaration. Add a program under test/aot that calls Buffer.of with each sort of operand and receiver.
There was a problem hiding this comment.
LGTM — the latest commit adds the Buffer.of declaration to buffer.builtins.ts, which closes the one item left open from the earlier review.
What was reviewed:
jsBufferConstructorFunction_ofinsrc/jsc/bindings/JSBuffer.cpp:RETURN_IF_EXCEPTIONfollows the allocation and everysetIndex(each can run uservalueOf/toPrimitive); the fresh buffer is unreachable from JS until the return, so a mid-loop throw just drops it, and the stack-held cell is rooted the same way as the siblingJSBuffer__bufferFromLengthAsArraypath.- The new
ofentry inbuffer.builtins.tsmirrors thealloc/concat"js" overloads (newBuffer,throws,runsOnlyOperandCode,converted), andb.any.restis the form used by the other variadic declarations;own-properties.txtand the AOT locked-members program/expected output were updated consistently. - Tests cover receiver independence, detached/callback use, subclass and Proxy non-construction, conversion order with a reentrant
valueOf, first-throw semantics (Symbol/BigInt TypeError), the 1000/1001 inline-storage boundary, and aBun.gc(true)stress insidevalueOf.
Extended reasoning...
The change adds an own of static to Node's Buffer constructor: one ~20-line JSC host function and a LUT row in src/jsc/bindings/JSBuffer.cpp, a matching declaration in the AOT builtins table and own-properties list under src/codegen/builtins/, plus 19 new bun:test cases and an AOT program. It touches no security-sensitive surface (no auth, crypto, parsing of untrusted bytes beyond ToNumber coercion of call arguments). The C++ follows the file's existing allocate-then-fill pattern with exception checks after each call that can enter JS, the behaviour change (Sub.of and Buffer.of.call(X) no longer construct through the receiver) is a deliberate match of Node's arrow-function implementation that the tests pin down, no CODEOWNERS entry covers the changed paths, and the only prior review item was addressed by the final commit.
main moved the AOT programs out of test/aot/programs. The Buffer.of program is now a file of its own under test/aot/parallel/builtin-modules, and the program about locked members is the IR test test/aot/ir/lower/programs/language/typedarray-buffer-classes-keep-members.js.
|
Merged main (3a9ea00) into this branch. The PR merges clean again.
|
There was a problem hiding this comment.
LGTM — the earlier nit is addressed: of is now declared in src/codegen/builtins/host/buffer.builtins.ts alongside the LUT row, so the AOT table has one source of truth for the Buffer statics.
What was reviewed:
jsBufferConstructorFunction_of:RETURN_IF_EXCEPTIONaftercreateUninitializedBufferand after everysetIndex(which runs ToPrimitive/ToNumber before its own bounds check); the buffer is a stack local never exposed to JS before return, so a throw drops it fully and GC duringvalueOfis covered by theBun.gc(true)test.- The four mirrored sites (LUT row,
own-properties.txtof/wc,buffer.builtins.tsentry, AOT lower-program member list) agree on an enumerable/writable/configurableofwithlength0. - Tests assert exact bytes, prototype, byteOffset and ArrayBuffer length via
toEqual, cover receivers, subclass/Proxy non-construction, argument ordering and throw-stop, and the 1000/1001 in-heap storage boundary.
Extended reasoning...
The diff adds a 15-line host function and LUT row in src/jsc/bindings/JSBuffer.cpp, one entry each in the AOT builtins declaration and own-properties table, a one-line update to an AOT program expectation, a new AOT program, and ~150 lines of tests in test/js/node/buffer.test.js. It touches no auth, crypto, parsing of external data or security-sensitive surface; the only native code is a bounded loop over call-frame arguments with exception checks after each call that can enter JS. The change is small, follows the file's existing createUninitializedBuffer/RETURN_IF_EXCEPTION conventions, has no CODEOWNERS entry, and the single open item from the prior review is resolved in the code itself; the hunt ended on a dry streak with no findings.
|
The IR test that was not run above passes now: |
One conflict, in src/codegen/builtins/data/own-properties.txt. main (#47942) added the line `BroadcastChannel.prototype open` directly above the `Buffer` line, and this branch adds `of/wc` to the `Buffer` line. The merge keeps both.
|
Merged main (2596ac5) into this branch. The PR has no conflict with main again.
|
Problem
Bufferhas no ownof, soBuffer.ofis the inherited%TypedArray%.of, which constructs throughthis.Promise.resolve(65).then(Buffer.of)rejects withTypeError: TypedArray.of requires |this| to be a constructor. Node gives<Buffer 41>.jsBufferConstructorTable(src/jsc/bindings/JSBuffer.cpp:3772) has noofrow. No user reported it. A comparison with Node found it during node:buffer: implement Buffer.from natively and share one body with the Buffer constructor #45285.Fix
of jsBufferConstructorFunction_of Function 0and a host function. It stores each argument in a newBufferand never readsthis, like Node's arrow function (lib/buffer.js).--aotbuilds, listof/wcinown-properties.txtand declare the function inbuffer.builtins.ts.test/js/node/buffer.test.js(19 new tests, 10 fail on bun 1.4.3) and one newtest/aotprogram. Adds no passing Node tests.Background
Buffer's statics. JSC makes a row a real property on its first read.bun build --compile --aotreads a table of built-ins.own-properties.txtlists each object's own properties, andbuffer.builtins.tsdeclares what eachBufferstatic does.new Buffer(n)per call) and an eager put infinishCreation(the only static outside the table).Downsides
Sub.of()runs no subclass constructor, andBuffer.of.call(Uint16Array, 1)returns aBuffer.Buffer.ofcan make JSC recompile optimizedBuffercallers. A probe: +28.9M instructions (main: 0)..text+256 B, file +4,096 B.Notes
Repro
Each line ran on Node v26.3.0, on bun main (017b243) and on this PR.
TypeErrorstands forTypeError: TypedArray.of requires |this| to be a constructor.const { of } = Buffer; of(1, 2)<Buffer 01 02>TypeError<Buffer 01 02>[65, 66].map(Buffer.of)[ <Buffer 41 00 00>, <Buffer 42 01 00> ]TypeErrorawait Promise.resolve(65).then(Buffer.of)<Buffer 41>TypeError<Buffer 41>Reflect.apply(Buffer.of, undefined, [1, 2, 3])<Buffer 01 02 03>TypeError<Buffer 01 02 03>Object.hasOwn(Buffer, "of")truefalsetrueObject.getOwnPropertyDescriptor(Buffer, "of"){ value: [Function: of], writable: true, enumerable: true, configurable: true }undefinedBuffer.of === Uint8Array.offalsetruefalseBuffer.of.call(Uint8Array, 1, 2)<Buffer 01 02>Uint8Array(2) [ 1, 2 ]<Buffer 01 02>class Sub extends BufferinSub.of(1, 2)require("safer-buffer").Buffer.of(1, 2)<Buffer 01 02>TypeError: safer.Buffer.of is not a function<Buffer 01 02>Who this reaches
test/node_modules/.bunreferencesBuffer.of.safer.jscopies the own enumerable statics ofBufferto a plain object. That copy needs an own enumerableofthat does not readthis.Why Node has its own
ofNode PR 19682 (
buffer: do not emit deprecation notice on Buffer.of) added it. The purpose was to keepBuffer.ofaway from the deprecatedBuffer()constructor (DEP0005). Bun does not emit DEP0005. The basis for this PR is the behaviour that Node shows: an own enumerable property, and a function that ignores the receiver.Behaviour changes
Each one now matches Node v26.3.0.
Sub.of(...)does not run the constructor of aBuffersubclass. If that constructor returns instances of the subclass,Sub.of()gave such an instance before. It gives a plainBuffernow.Buffer.of.call(X, ...)returns aBufferfor eachX. Before, it constructed throughX.Object.keys(Buffer), for-in andutil.inspect(Buffer)showof.Buffer.of !== Uint8Array.of. A patch ofUint8Array.ofdoes not changeBuffer.of.bun build --compile --aotlocks every own member ofBuffer. There, a store toBuffer.ofor adeleteof it throws, as forBuffer.from. Before,ofwas inherited and thedeletereturned true.Not changed
bun build --target=browserbundles forbuffer(src/node-fallbacks/buffer.js). ItsBuffer.ofis still the inherited%TypedArray%.of. A detachedBuffer.ofnow works in the runtime and still throws in a browser bundle.Buffer[Symbol.species]: own in Node, inherited in Bun. node:buffer: implement Buffer.from natively and share one body with the Buffer constructor #45285 changes it.Buffer.alloc.lengthis 1 (Node: 3),Buffer.from.nameandBuffer.isBuffer.nameare empty, andnew Buffer.concat(list)throws. Related work: Name JS builtin functions when their executable is first created (Buffer.isBuffer.namewas empty) #39559 (names of JS builtins) and Don't register native functions as their own constructors #34575 (native functions as constructors).Ahead-of-time builds
Main added
bun build --compile --aotand its table of built-ins in #45631, after this PR opened. #47129 then moved the AOT programs out oftest/aot/programs, and #47942 brought the rewritten compiler. The branch contains a merge of main (2596ac5).src/codegen/builtins/data/own-properties.txtlists the own properties of each built-in object. The generator of the table holds each row of a native property table against it. Withoutof/wcon theBufferline, the build stops withBuffer.of is declared and own-properties.txt does not have it.test/aot/builtins/own-properties.test.tscompares the list with theBufferof the built binary.src/codegen/builtins/host/buffer.builtins.tsdeclares each static ofBuffer. The new entry says thatBuffer.oftakes any number of operands, runs only their ToPrimitive, can throw, and returns a freshBuffer. Without it, the row gets the overload of an undeclared row (any result, any use of the operands), and the build listsBuffer.of: the result of Call is anything. The generated table differs in this one entry.Buffer. That check istest/aot/builtins/differential.test.tsnow (AOT_BUILTINS_FILTER='^Buffer\.of$').ofamong the inherited keys ofBuffer. It now hasofamong the members. After AOT: fixes for modules, classes and Workers, WebKit upgrade, shorter compiler docs #47129 it is the IR testtest/aot/ir/lower/programs/language/typedarray-buffer-classes-keep-members.js.test/aot/parallel/builtin-modules/buffer-of-with-every-sort-of-operand-and-receiver.jscallsBuffer.ofwith each sort of operand and receiver, in a loop too. It prints the same underbun <file>, under--compile --aotand under Node v26.3.0. On a release build of main at 11c3671 the program failed: the run underbun <file>stops at the detached call.--aotcame after it. The declaration answers a review comment on this PR.own-properties.txt. AOT: front end, test runs and tests for the rewritten compiler #47942 added the lineBroadcastChannel.prototype opendirectly above theBufferline. The merge keeps both lines.AOT_BUILTINS_UPDATE=1onown-properties.test.tsleaves the file as it is. The diff of this PR against main has the same lines as before that merge.test/aot/builtins/own-properties.test.ts: 16 pass. The IR test passes with the check of the engine (TESTAOT,AOT_IR_FILTER=typedarray-buffer-classes-keep-members cargo test -p bun_aot --test ir_tests).test/aot/run-parallel.tspasses the new program as written and with its types erased. The differential check ofBufferOf/0passes (AOT_BUILTINS_UPDATE=1 AOT_BUILTINS_FILTER='^Buffer\.of$'), and it fails for a table that says no ToPrimitive runs.Buffer.of, intest/aot/parallel/classes-of-views-that-are-only-constructed/. On main that call reaches%TypedArray%.ofwithBufferas the receiver. With this PR it reaches the rowBuffer.of. Both programs pass in both ways.views-made-by-from-and-of-are-printed-and-bring-little-of-the-library.mjscounts 8 forImageFunctionsEmitted, and its bound is 24. This PR does not edit the two programs.test/internal/builtins-table(119 pass, 1 skip, 7 fail on the debug build), 4 tests run the generator of the table with the Bun under test and reach their limit of 300 s. The other 3 failures follow from those. The build runs the same generator against the prebuilt engine, and that step passes.test/aot/builtins: 91 pass, 3 todo, 0 fail.test/internal/builtins-table: 133 pass, 1 skip.Measurements
Release builds on linux x64: main 017b243 (before #45631), and that commit with the
src/jscchange of this PR. The binary size on the current base is at the end of this list.perf,valgrind,bloatyandstraceare not installed in the build container. The instruction counts come from aperf_event_opencounter (PERF_COUNT_HW_INSTRUCTIONS, user space only, child threads included).nm -S,size -A):jsBufferConstructorFunction_ofis 446 B.jsBufferConstructorTableValuesgoes from 400 to 440 B.jsBufferConstructorTableIndexstays at 136 B..textgoes from 81,368,037 to 81,368,293 B (+256)..rodatadoes not change. The stripped file stays at 106,911,456 B.JSBuffer.lut.h:Values[10]becomesValues[11],Index[34]stays. One empty index slot now holds the row. No existing slot moves.bun:jscheapStats(), 3 identical runs): the first touch ofBufferadds 133 Function and 125 NativeExecutable cells on both builds. The first read ofBuffer.ofadds 1 and 1 (main: 0 and 0).Buffer.of(...)method call (slope from 200k to 1.2M iterations, empty loop subtracted, median of 3 runs):Buffer.isEncoding("utf8")304,Buffer.alloc(3)506,new Buffer(3)381,Uint8Array.of(1, 8, 15)624 (JIT off). With the JIT: 132, 322, 269 and 417.Buffer.of(1, 2, 3)call (gdb breakpoint counts over 1000 calls):Interpreter::executeConstruct1 to 0,createJSBufferFromJS1 to 0, typed-array allocations 1 to 1. A single-step of the host call: 774 to 348 instructions, 3 to 0 indirect calls, 13 to 0 instructions inmemset.Buffer.alloc,Buffer.isBufferandBuffer.byteLength. Then it reads one more static for the first time and runs the function again. Each phase has 2e6 calls. The JIT is not concurrent, and each number is the median of 5 runs. WithBuffer.ofas the late read, the function has 1 DFG compile on main and 2 on this PR, for +28.9M instructions. WithBuffer.concatas the late read, it has 2 on both builds, for +27.9M on main and +28.0M on this PR. The run-to-run spread is up to 3.6M..textgoes from 108,646,665 to 108,646,921 B (+256)..rodata,.dataand.bssdo not change. The stripped file goes from 138,193,048 to 138,197,144 B (+4,096, one page). On the first base the file size did not change.Tests
describe("Buffer.of")has 19 tests. On bun 1.4.3-canary (367d939), 10 fail and 9 pass. The 9 guard behaviour that must not change: the conversion order, a throw fromvalueOf, 0 to 5000 arguments, and a GC insidevalueOf.bun:testshim: 19 pass.buffer.test.jshas 739 pass, 1 skip, 0 fail on a debug build with ASAN. Before the first merge of main, the debug build with ASAN had 737 pass, 1 skip, 0 fail (main at that base: 718 pass). WithBUN_JSC_validateExceptionChecks=1, theBuffer.oftests report no unchecked scope. A build without the check aftersetIndexaborts under that flag withUnchecked JS exception.Buffer.ofpass on main and on this PR:test-buffer-of-no-deprecation.js,test-buffer-copy.js,test-string-decoder-end.js.string-decoder.test.jshas 143 pass.The uninitialized allocation
The host function allocates the buffer without a zero fill. JS cannot reach the buffer before the return, so the loop writes every byte or a throw drops the buffer. JSC's fast path for
Uint8Array.ofdoes the same.Alternatives weighed
new Buffer(n)on each call, which is the zero-filled path that this PR takes out ofBuffer.of.JSBufferConstructor::finishCreation. Each realm that createsBufferallocates the function, andofbecomes the only static outside the table. It removes the late first read cost forofonly. The other ten rows keep that cost.Uint8Array.of(typedArrayOfFast) also allocates uninitialized.%TypedArray%.ofin JSC. That function has the behaviour that the specification gives it.Related
git merge-file --diff3). The two can merge in either order. The body of node:buffer: implement Buffer.from natively and share one body with the Buffer constructor #45285 listsBuffer.ofunder "What stays different from Node". The PR that merges second must update that text.no test proof · iteration 8 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/buffer.test.js